fix(doctor): resolve settings-json and hooks-claude-count false positives - #821
Open
flaviohtavares wants to merge 2 commits into
Open
fix(doctor): resolve settings-json and hooks-claude-count false positives#821flaviohtavares wants to merge 2 commits into
flaviohtavares wants to merge 2 commits into
Conversation
…ives Both checks warned on a healthy framework checkout: - settings-json required >= 40 deny rules unconditionally, ignoring boundary.frameworkProtection. Contributors set it to false precisely so L1/L2 paths stay editable, making an empty deny list correct in that mode. Now reads the flag and PASSes when protection is off; defaults to protected when the config or key is absent. - hooks-claude-count only inspected settings.local.json, but this repo registers its hooks in settings.json, which Claude Code merges equally. Now collects commands from both files and reports how many of the discovered .cjs files are referenced, since engine hooks are spawned by their wrappers rather than registered directly. aiox doctor goes from 16 PASS / 2 WARN to 18 PASS / 0 WARN with no behaviour change for project installs. Tests: 8 new cases (59 total). Verified 5 of them fail against the unpatched checks; the other 3 are regression guards for the protected-by-default, explicit-true, and malformed-JSON paths. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
flaviohtavares
requested review from
Pedrovaleriolopez and
oalanicolas
as code owners
August 10, 2026 20:14
|
@flaviohtavares is attempting to deploy a commit to the SINKRA - AIOX Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
Welcome to aiox-core! Thanks for your first pull request. What happens next?
PR Checklist:
Thanks for contributing! |
github-actions
Bot
added
type: test
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThe doctor checks now read framework protection settings, support contributor mode, and inspect Claude hooks in both ChangesDoctor checks
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant HookCheck as hooks-claude-count.js
participant Settings as settings.json
participant LocalSettings as settings.local.json
HookCheck->>Settings: collectHookCommands
HookCheck->>LocalSettings: collectHookCommands
Settings-->>HookCheck: registered hook commands
LocalSettings-->>HookCheck: registered hook commands
HookCheck-->>HookCheck: report registration status
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.aiox-core/core/doctor/checks/hooks-claude-count.js:
- Around line 104-106: Update the hook filename matching in the referencedCount
calculation to require the complete .cjs filename at a command path or
shell-token boundary, rather than matching substrings or stripping the
extension. Preserve valid path references and add a regression test covering
similarly named hook files such as sync.cjs and sync-wrapper.cjs.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 766503bc-125c-4ec9-a1b7-660d58666362
📒 Files selected for processing (4)
.aiox-core/core/doctor/checks/hooks-claude-count.js.aiox-core/core/doctor/checks/settings-json.js.aiox-core/install-manifest.yamlpackages/installer/tests/unit/doctor/doctor-checks.test.js
Substring matching treated sync.cjs as registered when a command only referenced sync-wrapper.cjs, so a missing registration could produce a PASS with an inflated registered count. Commands are now tokenized: quotes and trailing shell punctuation are stripped, Windows separators normalized, and only complete .cjs basenames count as a reference. Addresses CodeRabbit review on SynkraAI#821. Tests: 3 new cases (62 total) — similarly named files, suffix-only references, and quoted/backslashed paths. Verified the first two fail against the previous substring matching. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two
aiox doctorchecks warn on a healthy framework checkout. Both are checker gaps rather than install problems — a freshly cloned, fully installed repo reports 16 PASS / 2 WARN when it should report 18 PASS / 0 WARN.settings-jsonThe check required
>= 40deny rules unconditionally and never consultedboundary.frameworkProtection. Contributors set that flag tofalseprecisely so L1/L2 paths stay editable — this repo ships it asfalseincore-config.yaml— which makes an empty deny list the correct state, not a misconfiguration.It now reads the flag and returns PASS when protection is off. The gate sits before both the deny-count test and the boundary-alignment test, so neither fires in contributor mode. Parsing is line-based to match the file's existing no-YAML-dependency style, and scoped to the
boundary:section so an unrelated key elsewhere can't be picked up. It defaults to protected when the config file or the key is absent, so project installs are unaffected.hooks-claude-countThe check inspected only
settings.local.json. This repo registers its hooks insettings.json, which Claude Code merges equally, so all five.cjsfiles were reported unregistered while actually being wired up and firing.It now collects hook commands from both files. The message reports how many of the discovered files are referenced (
5 hook files found, 3 registered) rather than implying all must be — the engine hooks are spawned as child processes by their wrappers (synapse-wrapper.cjs:23,precompact-wrapper.cjs:17) to isolate stdout/stderr, so registering them directly would break that isolation.Note that "fixing" this by duplicating the registrations into
settings.local.jsonwould be actively harmful: Claude Code merges both files, so every hook would fire twice per event.Tests
8 new cases, 59 total in
doctor-checks.test.js. Verified 5 fail against the unpatched checks; the other 3 are regression guards covering protected-by-default, explicitframeworkProtection: true, and malformed-JSON tolerance.Verification
npx jest packages/installer/tests/unit/doctor/doctor-checks.test.js— 59/59aiox doctor— 18 PASS / 0 WARN / 0 FAIL, was 16 PASS / 2 WARNNo behaviour change for project installs, where
frameworkProtectionistrueand hooks live insettings.local.json.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
New Features